-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Named pipes ota su #41708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Named pipes ota su #41708
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces named pipe communication for the OTA provider application, enabling external control, and adds a new Python test (TC_SU_2_8.py) for multi-fabric OTA scenarios. The changes span C++ and Python, modifying the OTA provider, adding new test infrastructure, and implementing the test case. My review has identified several issues, including a bug in the pipe cleanup logic, an unused variable in the C++ command handler, and incorrect API usage in the new Python test and its helper functions. Additionally, there is some commented-out code that should be removed for clarity. The comments regarding create_acl_entry in apps.py and apps.pyi have been removed as they refer to a type hint file update, which is outside the scope of code changes.
src/python_testing/matter_testing_infrastructure/matter/testing/matter_testing.py
Outdated
Show resolved
Hide resolved
examples/ota-provider-app/ota-provider-common/OtaProviderAppCommandDelegate.cpp
Outdated
Show resolved
Hide resolved
src/python_testing/matter_testing_infrastructure/matter/testing/event_attribute_reporting.py
Show resolved
Hide resolved
|
PR #41708: Size comparison from 1089161 to 1297b97 Full report (5 builds for cc32xx, realtek, stm32)
|
|
PR #41708: Size comparison from 1089161 to 3885c24 Full report (3 builds for realtek, stm32)
|
|
PR #41708: Size comparison from 1089161 to 0f891fe Full report (32 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds support for controlling and querying the OTA Provider example application via named pipes, which is useful for automated testing. It introduces a bidirectional communication channel to send commands (like QueryImageSnapshot) and receive responses. The changes span the OTA provider example app, the Linux platform layer for named pipes, and new Python test infrastructure.
My review found a couple of issues: a critical bug in OTAProviderExample where a member variable is reused incorrectly, and a bug in the cleanup logic for named pipes. Overall, the changes are a good step towards better testability of the OTA provider.
|
PR #41708: Size comparison from 1089161 to a424ffa Full report (5 builds for cc32xx, realtek, stm32)
|
Summary
Testing